Skip to content

fix(docker-outside-of-docker): check DOCKER_GID on container start - #1749

Open
Vladimir (varmiral) wants to merge 5 commits into
devcontainers:mainfrom
varmiral:main
Open

Vladimir (varmiral) wants to merge 5 commits into
devcontainers:mainfrom
varmiral:main

Conversation

@varmiral

@varmiral Vladimir (varmiral) commented Oct 6, 2026 •

Copy link
Copy Markdown

Problem

DOCKER_GID was computed once in install.sh at feature install time and baked into the generated /usr/local/share/docker-init.sh through heredoc expansion. This causes several problems when the container starts:

  1. Stale GID. The init script compares the socket GID against the docker group GID from build time, not the one in the running container. If the group GID differs at runtime, the comparison is wrong.
  2. Unnecessary socat proxy. When the socket GID already matched the docker group GID, the condition SOCKET_GID != DOCKER_GID was false and execution fell through to the else branch. That branch replaced docker.sock with a socat proxy even though nothing needed fixing.
  3. Fragile GID collision check. grep -E ".+:x:${SOCKET_GID}" /etc/group is not anchored to the GID field:
    • 100 also matches a group with GID 1000
    • it can match other fields of the line
    • it prints the matched line into the output

Changes

  • Removed the build-time DOCKER_GID="$(grep -oP ...)" from install.sh.
  • docker-init.sh now reads the docker GID at container start:
    DOCKER_GID="$(getent group docker | cut -d: -f3)"
  • Reworked the branching logic:
    • SOCKET_GID == DOCKER_GID → log it and do nothing (no proxy, no group changes)
    • SOCKET_GID != 0 and no group with that GID exists → groupmod --gid "${SOCKET_GID}" docker
    • otherwise (root-owned socket or GID already taken by another group) → fall back to the socat proxy, as before
  • Replaced the grep on /etc/group with getent group "${SOCKET_GID}". It does an exact lookup by GID and also works with NSS sources other than /etc/group.

…r the container restarts if the Docker GID has been changed

The DOCKER_GID variable must be defined every time the container starts; otherwise, after the Docker GID is changed via `sudoIf groupmod --gid "\${SOCKET_GID}" docker`, the socat fallback will trigger on subsequent restarts.
@varmiral
Vladimir (varmiral) requested a review from a team as a code owner October 6, 2026 20:10

@v-Kaniska244 Kaniska (v-Kaniska244) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Vladimir (@varmiral)

Thank you for the contribution. Would you please look into the failed tests. Also a version bump would be required for the feature.

@varmiral

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@varmiral

Copy link
Copy Markdown
Author

Kaniska (@v-Kaniska244) please forgive my inattention.

@v-Kaniska244 Kaniska (v-Kaniska244) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Vladimir (@varmiral)

LGTM. One last request if possible, could you please add some details in the PR description and modify the subject as it's currently truncated. That way this change will be better documented.

@varmiral Vladimir (varmiral) changed the title [docker-outside-of-docker] - The socat branch no longer executes afte… [docker-outside-of-docker] - Checking DOCKER_GID every time a container is started Oct 27, 2026
@varmiral Vladimir (varmiral) changed the title [docker-outside-of-docker] - Checking DOCKER_GID every time a container is started [docker-outside-of-docker] - Checking DOCKER_GID on every container launch Oct 27, 2026
@varmiral Vladimir (varmiral) changed the title [docker-outside-of-docker] - Checking DOCKER_GID on every container launch fix(docker-outside-of-docker): check DOCKER_GID on container start Oct 27, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants